[DO NOT SUBMIT] Contextual iFDO "demo" PR #86036

mtrofin · 2024-03-20T23:23:55Z

Main components of the contextual instrumented FDO work. This is a "demo" PR, to accompany the RFC, to give an overall idea of what's involved. The change will be broken down into smaller pieces, to be reviewed piecemeal. Test coverage is minimal in this demo PR.

github-actions · 2024-03-20T23:26:23Z

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff 65f07b804c2c05cf49bd043f2a6e9a0020198165 70b1c414aa7f6d2393aa659fc917056422066695 -- compiler-rt/lib/profile/InstrProfilingContextual.cpp compiler-rt/lib/profile/InstrProfilingContextual.h compiler-rt/lib/profile/tests/InstrProfilingContextualTest.cpp compiler-rt/lib/profile/tests/driver.cpp compiler-rt/test/profile/Linux/contextual.cpp llvm/include/llvm/ProfileData/CtxInstrProfileReader.h llvm/include/llvm/Transforms/Instrumentation/PGOCtxProfLowering.h llvm/lib/Transforms/Instrumentation/PGOCtxProfLowering.cpp llvm/tools/llvm-ctx-ifdo/llvm-ctx-ifdo.cpp llvm/include/llvm/IR/IntrinsicInst.h llvm/lib/IR/IntrinsicInst.cpp llvm/lib/Passes/PassBuilder.cpp llvm/lib/Passes/PassBuilderPipelines.cpp llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp

View the diff from clang-format here.

diff --git a/compiler-rt/lib/profile/InstrProfilingContextual.cpp b/compiler-rt/lib/profile/InstrProfilingContextual.cpp
index 3ba95c83f4..b89128d323 100644
--- a/compiler-rt/lib/profile/InstrProfilingContextual.cpp
+++ b/compiler-rt/lib/profile/InstrProfilingContextual.cpp
@@ -49,12 +49,11 @@ __sanitizer::SpinMutex AllContextsMutex;
 SANITIZER_GUARDED_BY(AllContextsMutex)
 __sanitizer::Vector<ContextRoot *> AllContextRoots;
 
-ContextNode * markAsScratch(const ContextNode* Ctx) {
-  return reinterpret_cast<ContextNode*>(reinterpret_cast<uint64_t>(Ctx) | 1);
+ContextNode *markAsScratch(const ContextNode *Ctx) {
+  return reinterpret_cast<ContextNode *>(reinterpret_cast<uint64_t>(Ctx) | 1);
 }
 
-template<typename T>
-T consume(T& V) {
+template <typename T> T consume(T &V) {
   auto R = V;
   V = {0};
   return R;
@@ -104,7 +103,8 @@ __thread char __Buffer[kBuffSize] = {0};
 
 #define TheScratchContext                                                      \
   markAsScratch(reinterpret_cast<ContextNode *>(__Buffer))
-__thread void *volatile __llvm_instrprof_expected_callee[2] = {nullptr, nullptr};
+__thread void *volatile __llvm_instrprof_expected_callee[2] = {nullptr,
+                                                               nullptr};
 __thread ContextNode **volatile __llvm_instrprof_callsite[2] = {0, 0};
 
 COMPILER_RT_VISIBILITY __thread ContextRoot
@@ -115,7 +115,7 @@ __llvm_instprof_slow_get_callsite(uint64_t Guid, ContextNode **InsertionPoint,
                                   uint32_t NrCounters, uint32_t NrCallsites) {
   auto AllocSize = ContextNode::getAllocSize(NrCounters, NrCallsites);
   auto *Mem = __llvm_instrprof_current_context_root->CurrentMem;
-  char* AllocPlace = Mem->tryAllocate(AllocSize);
+  char *AllocPlace = Mem->tryAllocate(AllocSize);
   if (!AllocPlace) {
     __llvm_instrprof_current_context_root->CurrentMem = Mem =
         Mem->allocate(getArenaAllocSize(AllocSize), Mem);
@@ -128,7 +128,7 @@ __llvm_instprof_slow_get_callsite(uint64_t Guid, ContextNode **InsertionPoint,
 
 COMPILER_RT_VISIBILITY ContextNode *
 __llvm_instrprof_get_context(void *Callee, GUID Guid, uint32_t NrCounters,
-                            uint32_t NrCallsites) {
+                             uint32_t NrCallsites) {
   if (!__llvm_instrprof_current_context_root) {
     return TheScratchContext;
   }
@@ -147,7 +147,8 @@ __llvm_instrprof_get_context(void *Callee, GUID Guid, uint32_t NrCounters,
   auto *Ret = Callsite ? Callsite
                        : __llvm_instprof_slow_get_callsite(
                              Guid, CallsiteContext, NrCounters, NrCallsites);
-  if (Ret->callsites_size() != NrCallsites || Ret->counters_size() != NrCounters)
+  if (Ret->callsites_size() != NrCallsites ||
+      Ret->counters_size() != NrCounters)
     __sanitizer::Printf("[ctxprof] Returned ctx differs from what's asked: "
                         "Context: %p, Asked: %lu %u %u, Got: %lu %u %u \n",
                         Ret, Guid, NrCallsites, NrCounters, Ret->guid(),
@@ -213,7 +214,7 @@ COMPILER_RT_VISIBILITY void __llvm_profile_reset_ctx_counters(void) {
 }
 
 COMPILER_RT_VISIBILITY
-int __llvm_ctx_profile_dump(const char* Filename) {
+int __llvm_ctx_profile_dump(const char *Filename) {
   __sanitizer::Printf("[ctxprof] Start Dump\n");
   __sanitizer::GenericScopedLock<__sanitizer::SpinMutex> Lock(
       &AllContextsMutex);
diff --git a/compiler-rt/lib/profile/InstrProfilingContextual.h b/compiler-rt/lib/profile/InstrProfilingContextual.h
index fa205a480e..3fd6ef0d93 100644
--- a/compiler-rt/lib/profile/InstrProfilingContextual.h
+++ b/compiler-rt/lib/profile/InstrProfilingContextual.h
@@ -29,7 +29,7 @@ public:
     return start() + (Pos - S);
   }
   Arena *next() const { return Next; }
-  const char *start() const { return const_cast<Arena*>(this)->start(); }
+  const char *start() const { return const_cast<Arena *>(this)->start(); }
   const char *pos() const { return start() + Pos; }
 
 private:
@@ -88,16 +88,11 @@ public:
 
   void reset();
 
-  void onEntry() {
-    ++counters()[0];
-  }
+  void onEntry() { ++counters()[0]; }
 
-  uint64_t entrycount() const {
-    return counters()[0];
-  }
+  uint64_t entrycount() const { return counters()[0]; }
 };
 
-
 // Exposed for test. Constructed and zero-initialized by LLVM. Implicitly,
 // LLVM must know the shape of this.
 struct ContextRoot {
@@ -108,7 +103,7 @@ struct ContextRoot {
   ::__sanitizer::StaticSpinMutex Taken;
 };
 
-inline bool isScratch(const ContextNode* Ctx) {
+inline bool isScratch(const ContextNode *Ctx) {
   return (reinterpret_cast<uint64_t>(Ctx) & 1);
 }
 
@@ -125,13 +120,13 @@ extern __thread __profile::ContextRoot
 
 COMPILER_RT_VISIBILITY __profile::ContextNode *
 __llvm_instrprof_start_context(__profile::ContextRoot *Root,
-                              __profile::GUID Guid, uint32_t Counters,
-                              uint32_t Callsites);
+                               __profile::GUID Guid, uint32_t Counters,
+                               uint32_t Callsites);
 COMPILER_RT_VISIBILITY void
 __llvm_instrprof_release_context(__profile::ContextRoot *Root);
 
 COMPILER_RT_VISIBILITY __profile::ContextNode *
 __llvm_instrprof_get_context(void *Callee, __profile::GUID Guid,
-                            uint32_t NrCounters, uint32_t NrCallsites);
+                             uint32_t NrCounters, uint32_t NrCallsites);
 }
 #endif
\ No newline at end of file
diff --git a/compiler-rt/lib/profile/tests/InstrProfilingContextualTest.cpp b/compiler-rt/lib/profile/tests/InstrProfilingContextualTest.cpp
index c5a0ddab23..7899f9a485 100644
--- a/compiler-rt/lib/profile/tests/InstrProfilingContextualTest.cpp
+++ b/compiler-rt/lib/profile/tests/InstrProfilingContextualTest.cpp
@@ -6,11 +6,11 @@
 using namespace __profile;
 
 TEST(ArenaTest, Basic) {
-  Arena * A = Arena::allocate(1024);
+  Arena *A = Arena::allocate(1024);
   EXPECT_EQ(A->size(), 1024U);
   EXPECT_EQ(A->next(), nullptr);
 
-  auto *M1 = A->tryAllocate(1020); 
+  auto *M1 = A->tryAllocate(1020);
   EXPECT_NE(M1, nullptr);
   auto *M2 = A->tryAllocate(4);
   EXPECT_NE(M2, nullptr);
@@ -56,7 +56,7 @@ TEST(ContextTest, Callsite) {
   EXPECT_EQ(Subctx->callsites_size(), 1U);
   EXPECT_EQ(__llvm_instrprof_expected_callee[0], nullptr);
   EXPECT_EQ(__llvm_instrprof_callsite[0], nullptr);
-  
+
   EXPECT_EQ(Subctx->size(), sizeof(ContextNode) + 3 * sizeof(uint64_t) +
                                 1 * sizeof(ContextNode *));
   __llvm_instrprof_release_context(&Root);
@@ -83,14 +83,14 @@ TEST(ContextTest, ScratchDuringCollection) {
   EXPECT_TRUE(isScratch(Subctx));
   EXPECT_EQ(__llvm_instrprof_expected_callee[0], nullptr);
   EXPECT_EQ(__llvm_instrprof_callsite[0], nullptr);
-  
+
   int ThirdOpaqueValue = 0;
   __llvm_instrprof_expected_callee[1] = &ThirdOpaqueValue;
   __llvm_instrprof_callsite[1] = &Subctx->subContexts()[0];
 
   auto *Subctx2 = __llvm_instrprof_get_context(&ThirdOpaqueValue, 3, 0, 0);
   EXPECT_TRUE(isScratch(Subctx2));
-  
+
   __llvm_instrprof_release_context(&Root);
 }
 
diff --git a/llvm/include/llvm/Transforms/Instrumentation/PGOCtxProfLowering.h b/llvm/include/llvm/Transforms/Instrumentation/PGOCtxProfLowering.h
index 975775f18a..fc9afc9ca2 100644
--- a/llvm/include/llvm/Transforms/Instrumentation/PGOCtxProfLowering.h
+++ b/llvm/include/llvm/Transforms/Instrumentation/PGOCtxProfLowering.h
@@ -27,7 +27,7 @@ private:
   Type *ContextNodeTy = nullptr;
   Type *ContextRootTy = nullptr;
 
-  DenseMap<const Function*, Constant*> ContextRootMap;
+  DenseMap<const Function *, Constant *> ContextRootMap;
   Function *StartCtx = nullptr;
   Function *GetCtx = nullptr;
   Function *ReleaseCtx = nullptr;
diff --git a/llvm/lib/IR/IntrinsicInst.cpp b/llvm/lib/IR/IntrinsicInst.cpp
index 9bf8dae894..9ee6dae30c 100644
--- a/llvm/lib/IR/IntrinsicInst.cpp
+++ b/llvm/lib/IR/IntrinsicInst.cpp
@@ -291,9 +291,7 @@ Value *InstrProfIncrementInst::getStep() const {
   return ConstantInt::get(Type::getInt64Ty(Context), 1);
 }
 
-Value *InstrProfCallsite::getCallee() const {
-  return getArgOperand(4);
-}
+Value *InstrProfCallsite::getCallee() const { return getArgOperand(4); }
 
 std::optional<RoundingMode> ConstrainedFPIntrinsic::getRoundingMode() const {
   unsigned NumOperands = arg_size();
diff --git a/llvm/lib/Transforms/Instrumentation/PGOCtxProfLowering.cpp b/llvm/lib/Transforms/Instrumentation/PGOCtxProfLowering.cpp
index 1212f7acf5..7685f2a00f 100644
--- a/llvm/lib/Transforms/Instrumentation/PGOCtxProfLowering.cpp
+++ b/llvm/lib/Transforms/Instrumentation/PGOCtxProfLowering.cpp
@@ -93,7 +93,7 @@ PreservedAnalyses PGOCtxProfLoweringPass::run(Module &M,
                          nullptr, "__llvm_instrprof_expected_callee");
   ExpectedCalleeTLS->setThreadLocal(true);
   ExpectedCalleeTLS->setVisibility(llvm::GlobalValue::HiddenVisibility);
-  
+
   for (auto &F : M)
     lowerFunction(F);
   return PreservedAnalyses::none();
@@ -111,7 +111,8 @@ void PGOCtxProfLoweringPass::lowerFunction(Function &F) {
       for (const auto &I : BB) {
         if (const auto *Incr = dyn_cast<InstrProfIncrementInst>(&I)) {
           if (!NrCounters)
-            NrCounters = static_cast<uint32_t>(Incr->getNumCounters()->getZExtValue());
+            NrCounters =
+                static_cast<uint32_t>(Incr->getNumCounters()->getZExtValue());
         } else if (const auto *CSIntr = dyn_cast<InstrProfCallsite>(&I)) {
           if (!NrCallsites)
             NrCallsites =
@@ -126,7 +127,7 @@ void PGOCtxProfLoweringPass::lowerFunction(Function &F) {
   Value *RealContext = nullptr;
 
   StructType *ThisContextType = nullptr;
-  Value* TheRootContext = nullptr;
+  Value *TheRootContext = nullptr;
   Value *ExpectedCalleeTLSAddr = nullptr;
   Value *CallsiteInfoTLSAddr = nullptr;
 
@@ -145,10 +146,9 @@ void PGOCtxProfLoweringPass::lowerFunction(Function &F) {
       auto Iter = ContextRootMap.find(&F);
       if (Iter != ContextRootMap.end()) {
         TheRootContext = Iter->second;
-        Context = Builder.CreateCall(
-            StartCtx,
-            {TheRootContext, Guid, Builder.getInt32(NrCounters),
-             Builder.getInt32(NrCallsites)});
+        Context = Builder.CreateCall(StartCtx, {TheRootContext, Guid,
+                                                Builder.getInt32(NrCounters),
+                                                Builder.getInt32(NrCallsites)});
       } else {
         Context =
             Builder.CreateCall(GetCtx, {&F, Guid, Builder.getInt32(NrCounters),
@@ -171,7 +171,7 @@ void PGOCtxProfLoweringPass::lowerFunction(Function &F) {
       break;
     }
   }
-  if(!Context) {
+  if (!Context) {
     dbgs() << "[instprof] Function doesn't have instrumentation, skipping "
            << F.getName() << "\n";
     return;

simpler testing

github-actions · 2024-03-25T16:24:53Z

⚠️ Python code formatter, darker found issues in your code. ⚠️

You can test this locally with the following command:

darker --check --diff -r 65f07b804c2c05cf49bd043f2a6e9a0020198165...70b1c414aa7f6d2393aa659fc917056422066695 compiler-rt/test/profile/lit.cfg.py llvm/test/lit.cfg.py

View the diff from darker here.

--- compiler-rt/test/profile/lit.cfg.py	2024-03-25 16:20:32.000000 +0000
+++ compiler-rt/test/profile/lit.cfg.py	2024-03-25 17:34:39.834042 +0000
@@ -157,14 +157,11 @@
         build_invocation(clang_cflags, True) + " -fprofile-instr-generate=",
     )
 )
 
 config.substitutions.append(
-    (
-        "%llvm-ctx-ifdo",
-        os.path.join(config.llvm_tools_dir, "llvm-ctx-ifdo")
-    )
+    ("%llvm-ctx-ifdo", os.path.join(config.llvm_tools_dir, "llvm-ctx-ifdo"))
 )
 
 if config.host_os not in [
     "Windows",
     "Darwin",

mtrofin added 3 commits March 20, 2024 15:45

WIP

24b1590

converter (raw -> bitstream)

215a7c6

bitstream profile data reader

603eccf

mtrofin added 3 commits March 21, 2024 16:31

disable vp when ctx prof; also don't emit callsite info unless ctx prof

7155748

simpler testing

Integration test and fixes

b586ca5

fix the output of the integration test

46ddc60

mtrofin added 2 commits March 25, 2024 17:25

in integration test, block any pre-inliner activity

05cbd1f

more on the test

70b1c41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[DO NOT SUBMIT] Contextual iFDO "demo" PR #86036

[DO NOT SUBMIT] Contextual iFDO "demo" PR #86036

Uh oh!

mtrofin commented Mar 20, 2024

Uh oh!

github-actions bot commented Mar 20, 2024 •

edited

Loading

Uh oh!

github-actions bot commented Mar 25, 2024 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

[DO NOT SUBMIT] Contextual iFDO "demo" PR #86036

Are you sure you want to change the base?

[DO NOT SUBMIT] Contextual iFDO "demo" PR #86036

Uh oh!

Conversation

mtrofin commented Mar 20, 2024

Uh oh!

github-actions bot commented Mar 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Mar 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

github-actions bot commented Mar 20, 2024 •

edited

Loading

github-actions bot commented Mar 25, 2024 •

edited

Loading